[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering `reinterpret_cast` #152610

Jaddyen · 2025-08-07T23:09:50Z

This patch lowers memref.reinterpret_cast.
From:

func.func @casting(%arg0: memref<999xi32>) {
  %reinterpret_cast_5 = memref.reinterpret_cast %arg0 to offset: [0], sizes: [1, 1, 999], strides: [999, 999, 1] : memref<999xi32> to memref<1x1x999xi32>
  return
}

To:

void casting(int32_t v1[999]) {
  std::size_t v2 = 0;
  int32_t* v3 = &v1[v2];
  int32_t(*v4)[1][1][999]  = reinterpret_cast<int32_t(*)[1][1][999]>(v3);
  return;
}

ilovepi · 2025-08-08T17:23:33Z

In your example, int32_t(*)[1][1][999] v4 is invalid syntax, right? Shouldn't it be: int32_t(*v4)[1][1][999]? Is that a typo in the description? or is that what's generated/translated?

ilovepi · 2025-08-08T18:01:25Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+    emitc::ConstantOp zeroIndex = rewriter.create<emitc::ConstantOp>(
+        loc, rewriter.getIndexType(), rewriter.getIndexAttr(0));


I think I've seen a zeroIndex crated in several places now. Its not much code, but that may be a good candidate for a helper function (e.g. in the anonymous namespace and marked static).

ilovepi · 2025-08-08T18:02:14Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+      llvm::SmallVector<mlir::Value> indices;
+      for (int i = 0; i < rank; ++i) {
+        indices.push_back(zeroIndex);
+      }


Suggested change

llvm::SmallVector<mlir::Value> indices;

for (int i = 0; i < rank; ++i) {

indices.push_back(zeroIndex);

}

llvm::SmallVector<mlir::Value> indices(rank, zeroIndex);

ilovepi · 2025-08-08T18:02:34Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+    auto createPointerFromEmitcArray =
+        [loc, &rewriter, &zeroIndex](
+            mlir::TypedValue<emitc::ArrayType> arrayValue) -> emitc::ApplyOp {
+      int64_t rank = arrayValue.getType().getRank();
+      llvm::SmallVector<mlir::Value> indices;
+      for (int i = 0; i < rank; ++i) {
+        indices.push_back(zeroIndex);
+      }


This closure seems familar from your other patches. I'm guessing that means it should probably be a helper function instead, since I don't see anything in the capture list that couldn't be a parameter.

ilovepi · 2025-08-08T18:13:00Z

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-reinterpret-cast.mlir

+//CHECK-NEXT:     %1 = "emitc.constant"() <{value = 0 : index}> : () -> index
+//CHECK-NEXT:     %2 = emitc.subscript %0[%1] : (!emitc.array<999xi32>, index) -> !emitc.lvalue<i32>
+//CHECK-NEXT:     %3 = emitc.apply "&"(%2) : (!emitc.lvalue<i32>) -> !emitc.ptr<i32>
+//CHECK-NEXT:     %4 = emitc.call_opaque "reinterpret_cast"(%3) {args = [0 : index], template_args = [!emitc.ptr<!emitc.array<1x1x999xi32>>]} : (!emitc.ptr<i32>) -> !emitc.ptr<!emitc.array<1x1x999xi32>>


an actual reinterpret_cast<T>() is appropriate for C++, but for C, you'd need a C-style cast. given that the C-style cast will work in both languages and what you're doing can't be covered by static or dyn casts, maybe we should use a normal cast operation? Would that work here, do you think?

+1, good to check.

jpienaar · 2025-08-11T03:23:11Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+        indices.push_back(zeroIndex);
+      }
+
+      emitc::SubscriptOp subPtr = rewriter.create<emitc::SubscriptOp>(


If you switch to the new builder form (emit::SubscriptOp::create(...)) you should no longer need ValueRange here.

jpienaar · 2025-08-11T03:23:42Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+    auto castCall = rewriter.create<emitc::CastOp>(
+        loc, emitc::PointerType::get(targetInEmitC), srcPtr.getResult());
+
+    rewriter.replaceOp(castOp, castCall);


You can combine this with the preceding by doing rewriter.replaceWithNewOp

jpienaar · 2025-08-11T03:24:32Z

mlir/lib/Target/Cpp/TranslateToCpp.cpp

@@ -1756,6 +1756,20 @@ LogicalResult CppEmitter::emitOperation(Operation &op, bool trailingSemicolon) {

 LogicalResult CppEmitter::emitVariableDeclaration(Location loc, Type type,
                                                  StringRef name) {
+  if (auto pType = dyn_cast<emitc::PointerType>(type)) {


Could this be a standalone PR? (this feels like it is a general refinement).

jpienaar · 2025-08-11T03:26:02Z

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-reinterpret-cast.mlir

+//CHECK-NEXT:     %1 = "emitc.constant"() <{value = 0 : index}> : () -> index
+//CHECK-NEXT:     %2 = emitc.subscript %0[%1] : (!emitc.array<999xi32>, index) -> !emitc.lvalue<i32>
+//CHECK-NEXT:     %3 = emitc.apply "&"(%2) : (!emitc.lvalue<i32>) -> !emitc.ptr<i32>
+//CHECK-NEXT:     %4 = emitc.call_opaque "reinterpret_cast"(%3) {args = [0 : index], template_args = [!emitc.ptr<!emitc.array<1x1x999xi32>>]} : (!emitc.ptr<i32>) -> !emitc.ptr<!emitc.array<1x1x999xi32>>


+1, good to check.

jpienaar · 2025-08-11T03:30:35Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+      return rewriter.notifyMatchFailure(castOp.getLoc(),
+                                         "cannot convert memref type");
+    }
+    Location loc = castOp.getLoc();


Empty lines are for me similar to paragraphs in text, it creates logical separations/groupings which aid reading. Here I can't quite figure those out.

EtoAndruwa · 2025-08-13T09:44:35Z

mlir/test/Conversion/MemRefToEmitC/memref-to-emitc-reinterpret-cast.mlir

@@ -0,0 +1,16 @@
+// RUN: mlir-opt -convert-memref-to-emitc %s -split-input-file | FileCheck %s


Suggested change

// RUN: mlir-opt -convert-memref-to-emitc %s -split-input-file | FileCheck %s

// RUN: mlir-opt -convert-memref-to-emitc %s | FileCheck %s

The -split-input-file option must be dropped since there is no need to use it. Test includes only one function.

EtoAndruwa · 2025-08-13T09:57:11Z

mlir/lib/Conversion/MemRefToEmitC/MemRefToEmitC.cpp

+    auto srcArrayValue =
+        cast<TypedValue<emitc::ArrayType>>(adaptor.getSource());
+
+    emitc::ConstantOp zeroIndex = rewriter.create<emitc::ConstantOp>(


Using emitc::Literal can reduce emitted code by avoiding extra creation of constants, as the literal's value is emitted directly in place.

Jaddyen added 2 commits August 6, 2025 20:18

needs improvment

c6ea995

almost functional option

d09ccc0

Jaddyen requested review from jpienaar and ilovepi August 7, 2025 23:15

ilovepi reviewed Aug 8, 2025

View reviewed changes

cpp output that compiles

19e23d5

jpienaar reviewed Aug 11, 2025

View reviewed changes

EtoAndruwa reviewed Aug 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering `reinterpret_cast` #152610

[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering `reinterpret_cast` #152610

Jaddyen commented Aug 7, 2025 •

edited

Loading

Uh oh!

ilovepi commented Aug 8, 2025

Uh oh!

ilovepi Aug 8, 2025

Uh oh!

ilovepi Aug 8, 2025

Uh oh!

ilovepi Aug 8, 2025

Uh oh!

ilovepi Aug 8, 2025

Uh oh!

jpienaar Aug 11, 2025

Uh oh!

jpienaar Aug 11, 2025

Uh oh!

jpienaar Aug 11, 2025

Uh oh!

jpienaar Aug 11, 2025

Uh oh!

jpienaar Aug 11, 2025

Uh oh!

jpienaar Aug 11, 2025

Uh oh!

EtoAndruwa Aug 13, 2025

Uh oh!

EtoAndruwa Aug 13, 2025

Uh oh!

Uh oh!

		emitc::ConstantOp zeroIndex = rewriter.create<emitc::ConstantOp>(
		loc, rewriter.getIndexType(), rewriter.getIndexAttr(0));

		@@ -0,0 +1,16 @@
		// RUN: mlir-opt -convert-memref-to-emitc %s -split-input-file \| FileCheck %s

[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering reinterpret_cast #152610

Are you sure you want to change the base?

[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering reinterpret_cast #152610

Conversation

Jaddyen commented Aug 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ilovepi commented Aug 8, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering `reinterpret_cast` #152610

[mlir][EmitC] Expand the MemRefToEmitC pass - Lowering `reinterpret_cast` #152610

Jaddyen commented Aug 7, 2025 •

edited

Loading